Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the CI workflow for Node.js projects by separating SonarQube Cloud scanning into a dedicated job. Previously, the SonarQube scan ran as a conditional step within the tests job. The new structure creates a separate sonarcloud job that depends on the tests job, with coverage reports passed between jobs using GitHub Actions cache.
Changes:
- Separated SonarQube Cloud scanning into a dedicated job that runs after tests complete
- Added caching mechanism to transfer coverage reports from tests job to sonarcloud job
- Added path override logic to fix SonarQube source path warnings for Node.js coverage files
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path: ${{ (inputs.app-directory == '.' && inputs.working-directory || inputs.app-directory) && format('{0}/', inputs.app-directory == '.' && inputs.working-directory || inputs.app-directory) || ''}}coverage | ||
| key: cache-${{ github.run_id }}-${{ github.run_attempt }} | ||
|
|
||
| sonarcloud: | ||
| name: Run SonarQube Cloud Scan | ||
| if: ${{ !inputs.skip_sonar }} | ||
| runs-on: ubuntu-latest | ||
| needs: tests | ||
| defaults: | ||
| run: | ||
| working-directory: ${{ inputs.working-directory }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: Restore coverage report | ||
| uses: actions/cache/restore@9255dc7a253b0ccc959486e2bca901246202afeb # v5.0.1 | ||
| with: | ||
| path: ${{ (inputs.app-directory == '.' && inputs.working-directory || inputs.app-directory) && format('{0}/', inputs.app-directory == '.' && inputs.working-directory || inputs.app-directory) || ''}}coverage |
There was a problem hiding this comment.
The complex path construction expression ${{ (inputs.app-directory == '.' && inputs.working-directory || inputs.app-directory) && format('{0}/', inputs.app-directory == '.' && inputs.working-directory || inputs.app-directory) || ''}}coverage is duplicated between lines 179 and 197. This duplication makes the workflow harder to maintain and increases the risk of inconsistencies if one instance is updated but not the other. While GitHub Actions doesn't support workflow-level variables for reusable expressions, consider adding a comment explaining the logic or simplifying the expression if possible.
.github/workflows/ci-node.yml
Outdated
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
The fetch-depth: 0 is required for SonarQube Cloud scanning but was added to the tests job. Since the SonarQube scan has been moved to a separate job that already has its own checkout with fetch-depth: 0 (line 191-193), this setting in the tests job is now redundant. Consider removing it from the tests job checkout to speed up the checkout process, unless there's another reason the full git history is needed in the tests job.
| with: | |
| fetch-depth: 0 |
There was a problem hiding this comment.
Removed the mentioned lines as suggested.
2 participants
Move sonarcoud to a separate job.
Refs: RAT-358 https://helsinkisolutionoffice.atlassian.net/browse/RAT-358
Similar change done to backend: #24
There are two PR:s where I used a copy of this .yml to test that sonar is run and uses the coverage info.
These have in purpose new functios that don't have 100% test coverage. I will delete these later.
City-of-Helsinki/open-city-profile-ui#492
City-of-Helsinki/linkedcomponents-ui#561